Skip to content

Update code quality configuration #555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
khaeru merged 6 commits into from
Jan 20, 2025
Merged

Update code quality configuration #555

khaeru merged 6 commits into from
Jan 20, 2025

Conversation

@khaeru
Copy link
Member

@khaeru khaeru commented Jan 16, 2025 •
edited
Loading

  • Bump mypy, ruff versions and update config.
    • We now follow the configuration recommended in Using mypy via pre-commit python/mypy#13916, such that all files are checked with mypy, even if only a few are edited in a particular commit. This, for instance, avoids the situation that a change to type hints in one file causes typing errors in other files that are not checked.
    • IPython/Jupyter notebooks are now included in ruff formatting.
  • Update copyright year.
  • Work around Tags no longer fetch with Git v2.48.0 actions/checkout#2041.

How to review

TBD

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation. N/A, formatting only
  • Update release notes.

@khaeru khaeru added enh New features & functionality ci Continuous integration labels Jan 16, 2025
@khaeru khaeru self-assigned this Jan 16, 2025
@codecov
Copy link

codecov bot commented Jan 16, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.9%. Comparing base (3fa1191) to head (96407b5).
Report is 9 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##            main    #555     +/-   ##
=======================================
- Coverage   98.9%   98.9%   -0.1%     
=======================================
  Files         44      44             
  Lines       4790    4790             
=======================================
- Hits        4740    4738      -2     
- Misses        50      52      +2
Files with missing lines Coverage Δ
ixmp/backend/jdbc.py 97.2% <100.0%> (ø)
ixmp/cli.py 100.0% <ø> (ø)
ixmp/tests/backend/test_jdbc.py 100.0% <ø> (ø)
ixmp/tests/core/test_scenario.py 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@khaeru
Copy link
Member Author

khaeru commented Jan 16, 2025 •
edited
Loading

@glatterf42 I could, optionally, expand this PR with 2 kinds of improvements to the GitHub Actions workflows that I have prototyped in genno and elsewhere:

  1. pytest.yaml: use uv to set up Python and invoke pytest and pre-commit. As advertised, this seems to be very fast.

    It doesn't require any changes to code or packaging, so it seems to be a no-regret change.

  2. publish.yaml: use uv build and uv publish instead of other tools; drop chaining to our iiasa/actions reusable workflow; use PyPI×GitHub "trusted publishing".

    However, because of upstream issues (Uploads fail due to setuptools using wrong metadata version for license-file astral-sh/uv#9513 → [BUG] tool.setuptools.license-files results in invalid metadata pypa/setuptools#4759) this requires a change in the build-backend, for which I went with hatchling as suggested in comments on those issues. This isn't a major change, especially as I only used the build backend and did not attempt to use any of the other features that hatch advertises. But I think it begins to collide with our discussion of whether to adopt hatch, pdm, poetry, or something else, where we still don't have a clear "yes" on any option. Also, uv publish is marked experimental for the time being.

    So I would maybe avoid to do this one in ixmp and the rest of the stack, for the moment.

Please let me know briefly what you think, and I'll proceed and also make parallel changes to message_ix.

@glatterf42
Copy link
Member

glatterf42 commented Jan 17, 2025

Thanks for this PR :)

I agree with what you say, 1. sounds good, 2. can likely wait a little. Though I have already thought about migrating our package management to uv completely myself, i.e. migrate the pyproject.toml, generate a lock file, etc. But this can also wait until we can do 2. at least, I think.

@khaeru khaeru force-pushed the enh/code-quality branch 3 times, most recently from e3803b6 to ff7ba2a Compare January 20, 2025 10:51
khaeru added 5 commits January 20, 2025 12:13
@khaeru
Bump mypy, ruff versions
2d45f52 
- ruff v1.9.0 → v1.14.1.
  - Drop custom hook entry that uses PRE_COMMIT_MYPY_VENV.
  - Use config suggested at python/mypy#13916.
- ruff v0.3.4 → v0.9.1
  - Reformat 4 files.
  - Temporarily exclude *.ipynb files covered by new version.
@khaeru
Include *.ipynb in ruff format checking
48989cc 
Format 2 files.
@khaeru
Update copyright year 2024 → 2025
5bf0bfc
@khaeru
Use astral-sh/setup-uv in "pytest" GHA workflow
d0efe14 
- Discard commented workflow contents for pandas 2.0.0 pre-release.
- Reduce scope of exclusion for ts-graphviz/setup-graphviz#630
  workaround.
- Use Python location from setup-uv instead of $pythonLocation.
- Drop workaround for pyam-iamc exclusion of Python 3.13.
- Add --durations=20 to pytest invocation.
- Drop workaround for codecov/codecov-action#1316.
@khaeru
Work around actions/checkout#2041
1e260eb 
Borrowed from celeritas-project/celeritas#1580
@khaeru khaeru force-pushed the enh/code-quality branch 2 times, most recently from 40cfa18 to ab33b54 Compare January 20, 2025 11:56
@khaeru
Simplify installation of reticulate
96407b5 
- Drop use of "remotes"; call install.packages() directly.
- Install libpng-dev for compiling R 'png' package from source on Linux.
- Drop workaround for actions/runner-images#11137; appears no longer necessary.
@khaeru
Copy link
Member Author

khaeru commented Jan 20, 2025

Okay, now in place. Compared to this run:

  • macOS:
    • actions/setup-python: 2:49–5:33
    • astral-sh/setup-uv: 4:11–5:07
  • Linux:
    • actions/setup-python: 2:29–4:30
    • astral-sh/setup-uv: 2:38–4:08
  • Windows:
    • actions/setup-python: 13:08–15:58
    • astral-sh/setup-uv: 11:39–12:19

So only clear daylight on Windows, but by a few minutes. However there's a lot of variability in the unrelated steps. For the ubuntu-latest-py3.13 jobs, the times for the setup step + "Install the package and dependencies":

  • actions/setup-python: 11 seconds + 53 seconds.
  • astral-sh/setup-uv: 2 seconds + 6 seconds.

So that's a clear improvement.

@khaeru khaeru requested a review from glatterf42 January 20, 2025 12:22
@khaeru khaeru added this to the 3.10 milestone Jan 20, 2025
glatterf42
glatterf42 approved these changes Jan 20, 2025
View reviewed changes
Copy link
Member

@glatterf42 glatterf42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks great to me :)
Hopeful that this will stabilize our CI :)

@khaeru khaeru merged commit 58e801c into main Jan 20, 2025
21 checks passed
@khaeru khaeru deleted the enh/code-quality branch January 20, 2025 13:42
This was referenced Jan 21, 2025
Merged
Merged
@khaeru khaeru mentioned this pull request Sep 15, 2025
Open
@khaeru khaeru mentioned this pull request Nov 27, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glatterf42 glatterf42 glatterf42 approved these changes

Assignees

@khaeru khaeru

Labels

ci Continuous integration enh New features & functionality

Projects

None yet

Milestone

3.10

Development

Successfully merging this pull request may close these issues.

2 participants

@khaeru @glatterf42